Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable keyboard input in "split" mode #51

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zorun
Copy link

@zorun zorun commented Jul 12, 2014

Keyboard input does not seem useful in split mode, as the output is
probably meant to be parsed by another program. Users can press ^C anyway
if they want to interrupt mtr.

It was helplessly broken anyway: no key press was actually taken into
account (see bug #30 on Github), except the "Enter" key, which would
cause an infinite loop by calling select() repeatedly.

Keyboard input does not seem useful in split mode, as the output is
probably meant to be parsed by another program.  Users can press ^C anyway
if they want to interrupt mtr.

It was helplessly broken anyway: no key press was actually taken into
account (see bug traviscross#30 on Github), *except* the "Enter" key, which would
cause an infinite loop by calling select() repeatedly.
@zorun
Copy link
Author

zorun commented Jul 12, 2014

Btw, it might seem quite unhelpful to remove a feature just because I don't like it.

This actually caused me quite a lot of trouble: when using mtr in split mode in a bash script, it would randomly enter an infinite loop. I'm not quite sure why it didn't happen all the time, but this was annoying enough to debug.

So, this feature causes mtr to misbehave, and does not work anyway (see #30): it seems simpler to just remove it.

@rewolff
Copy link
Collaborator

rewolff commented Jul 12, 2014

It may help to understand what "split mode" is intended to do.

Although mtr security issues are "rare and far between", the situation is still that mtr is linked against several large libraries. Any C++ code in there will have initializers that are called BEFORE main is started. Main will allocate the sockets and drop root privileges. Any bugs in the (library) startup code are a possible security bug.

So the intention of "split" mode is to split mtr into a core that does the ping-ing, and several helper modules that

  • lookup DNS
  • aggregate/store the data
  • display the data.

The "split mode" is the first attempt at making the old mtr behave as the "pinging" module.

The intention is not to loose functionality while moving to the new architecture. And "display-control" modules are currently able to signal "please reset the aggregation" or "please move to a new target". So while "keyboard" input is not the intention of "split mode", monitoring the input for control messages IS.

@zorun
Copy link
Author

zorun commented Jul 12, 2014

Ah, right, thanks for the precision :)

In this case, it will indeed be a better idea to fix input handling, especially the infinite loop. I have absolutely no idea what's going wrong, but it seems related to ncurses.

@yvs2014
Copy link

yvs2014 commented Jul 19, 2014

Probably you need to have the terminal initialized (initscr()/newterm()) before called ncurses getch() function.
yvs2014@b476754
This works for me, but this is not cross-platform.

@cvidler
Copy link

cvidler commented Jul 8, 2016

zorun, thank you this was driving me crazy. bash script I was writing would work in any other mode than --split, but I wanted the --split output format.

I wasn't getting a random infinite loop, it'd do it every execution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants